Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added AlphaCube group to outputlabel in the event the image is cropped. #645

Merged
merged 18 commits into from
Feb 7, 2019

Conversation

twilson271828
Copy link
Contributor

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation change (update to the documentation; no code change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read and agree to abide by the Code of Conduct
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

  • I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

twilson271828 and others added 14 commits December 14, 2018 15:01
* Upgrade libtiff to 4.0.10 (DOI-USGS#636)

* Switch libtiff to 4.0.9 or higher to remove geotiff conflict
…cted images and screwing things up. Fixed it.
Fixed warning in Pixel unit tests
* Moved ISIS3 conda-build recipe from ISIS3_deps repository

* Un-pinned non-astro build numbers

* Removing build numbers from external libraries in the environment and meta.yeml files

* Final merging
DOI-USGS#5187 (DOI-USGS#659)

* Removed bolding of some text to decrease distraction.

* Fixed some typos.

* Reworded documentation.
@jlaura
Copy link
Collaborator

jlaura commented Dec 20, 2018

@scsides Can you put some comments onto this PR about areas that are of testing concern? The other contributors on this repo can then weigh in on methods of testing those areas.

@twilson271828
Copy link
Contributor Author

@jlaura I pinged Jesse to review this since I based it on his stats app tests.

@scsides
Copy link
Contributor

scsides commented Dec 21, 2018

@USGS-Astrogeology/developers
This Hayabusa2 sprint is currently envolves two types of tests:

  1. unit test for the camera model
  2. application tests for the ingestion and calibration programs

For the unit tests, it seems reasonable to convert the test to a gtest. However, all of the camera tests in ISIS3 use cube files. I will be attempting to minimize the size of these files first and looking into what it takes to eliminate them. The labels are what is actually used, so mocking them seems reasonable.

The application tests for both the ingestion program and the calibration program also require cube files at this time. Converting these tests to gtest is not a trivial effort, and there is reasonable concern regarding mocking of complex files. The mocking of PDS files will require changes to the ProcessImportPDS class. Not impossible but needs to be well thought out.

Thoughts:
Test only label output not cube DNs. NOT a good idea for some cases
Mock the input and output cubes and test a subset of DNs, mostly edge cases
This is a big problem that needs far more attention outside of little projects
The testing sprints did not solve these problems
Reducing the test data volume is a high priority. Do it carelessly could be costly.

Tyler Wilson and others added 2 commits January 1, 2019 20:09
…esolved symbol errors (CMakeLists.txt) and to fix some bugs in hyb2onc2isis involved with turning it into a callable function.
Copy link
Contributor

@jessemapel jessemapel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scsides I agree that there is going to need to be some amount of file to test this. Between the Process classes and how the calibration program uses Cubes, it would take a significant amount of work to get completely away from them.

On the ingestion app:
I think that 90% of it can be tested via gtest. All of the label translation and AlphaCube group creation can be split into separate functions that don't require any files and can be easily tested by gtest. This way all of the valid input cases can be identified from the SIS and then tested them without needing lots of different input files. The remainder of the ingestion app is then ImportFits behavior. I don't think it's up to this app's test to cover that. I'm inclined to cover it with at least one test of Hayabusa2 because I don't think ImportFits is properly covered, but this app doesn't do anything special with it.

On the calibration app:
About half of the logic tested without any actual files.

In Hyb2OncCalUtils.h:

  • isis2mat is already testable with a mock cube
  • mat2isis and translate can be modified so they take a pointer to an output cube instead of an output file name. Then, the output cube can be mocked.

In the application main the bulk of the code will be hard to test in gtest. The utility functions and the process function could be tested but the app would need to be completely refactored to remove global variables. I think it's a good idea to eventually remove all of those global variables, I'm not sure if there is time for that at the moment though.

On the camera unit tests:
Right now cameras cannot be tested without an actual image. I don't think there's anything we can do here at the moment.

All in all, I think there's a few places some things can be made more testable without much work, but a lot of it will still need to be tested by just running the apps.

@twilson271828 twilson271828 merged commit 925d940 into DOI-USGS:hayabusa2_fy19 Feb 7, 2019
acpaquette pushed a commit to acpaquette/ISIS3 that referenced this pull request Apr 6, 2021
…d. (DOI-USGS#645)

* Added AlphaCube group to outputlabel in the event the image is cropped.

* Fix libtiff dependency (DOI-USGS#636) (DOI-USGS#644)

* Upgrade libtiff to 4.0.10 (DOI-USGS#636)

* Switch libtiff to 4.0.9 or higher to remove geotiff conflict

* Moved ISIS3 conda-build recipe from ISIS3_deps repository (DOI-USGS#650)

* Fixed warning in Pixel unit tests

* Made a tweak to the dimensions of the alpha cube for cropped images to give it the correct dimensions.

* Changed the transform function so it could process an array of doubles instead of an array of ints.

* Smear correction was being incorrectly applied to onboard smear corrected images and screwing things up.  Fixed it.

* Added gtest capability to hyb2onc2isis

* Removing build numbers from external libraries (DOI-USGS#660)

* Moved ISIS3 conda-build recipe from ISIS3_deps repository

* Un-pinned non-astro build numbers

* Removing build numbers from external libraries in the environment and meta.yeml files

* Final merging

* Added pixel type attribute to the output image of program shadow. Fixes DOI-USGS#5187 (DOI-USGS#659)

* Removed bolding of some text to decrease distraction.

* Fixed some typos.

* Reworded documentation.

* Added section for Environment and PreferemcesSetup in the Getting Started Section. (DOI-USGS#663)

* Updated .gitmodules to use https rather than ssh (DOI-USGS#673)

* Added build type release to conda recipe (DOI-USGS#676)

* Modified isis/tests/CMakeLists.txt as well as hyb2onc2isis to fix unresolved symbol errors (CMakeLists.txt) and to fix some bugs in hyb2onc2isis involved with turning it into a callable function.

* Added w1 test to hyb2onc2isisTests.cpp

* Modified hyb2onccal to be a callable function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants